[Interactive Graph] Add logarithm graph scoring#3424
Conversation
🗄️ Schema Change: No Changes ✅ |
npm Snapshot: PublishedGood news!! We've packaged up the latest commit from this PR (886aa19) and published it to npm. You Example: pnpm add @khanacademy/perseus@PR3424If you are working in Khan Academy's frontend, you can run the below command. ./dev/tools/bump_perseus_version.ts -t PR3424If you are working in Khan Academy's webapp, you can run the below command. ./dev/tools/bump_perseus_version.js -t PR3424 |
|
Size Change: +42 B (+0.01%) Total Size: 497 kB
ℹ️ View Unchanged
|
🛠️ Item Splitting: No Changes ✅ |
57320af to
7c67738
Compare
There was a problem hiding this comment.
Claude Code Review
This repository is configured for manual code reviews. Comment @claude review to trigger a review and subscribe this PR to future pushes, or @claude review once for a one-time review.
Tip: disable this comment in your organization's Code Review settings.
|
@claude review once |
There was a problem hiding this comment.
LGTM — straightforward addition that mirrors the exponential scoring pattern exactly, with solid test coverage.
Extended reasoning...
Overview
The PR adds logarithm graph scoring to score-interactive-graph.ts by importing getLogarithmCoefficients from kmath and inserting a new else if block following the existing exponential scoring block verbatim. A changeset file and 6 new test cases (invalid/correct/incorrect/equivalent-curve) are also included.
Security risks
None. This is pure scoring logic — no auth, no I/O, no user-controlled code paths beyond the already-gated coefficient computation.
Level of scrutiny
Low. The change is mechanical and self-contained: it adds ~30 lines of logic that are a direct copy of the adjacent exponential scoring block, only substituting getLogarithmCoefficients for getExponentialCoefficients. The pattern is already well-established and tested throughout the file.
Other factors
Test coverage is thorough for the new path (undefined guess, missing coords, missing asymptote, correct match, wrong match, and equivalent-curve normalization). Bundle size impact is negligible (+42 B). No CODEOWNERS concerns for this file.
76ec318 to
665cffd
Compare
7c67738 to
8976066
Compare
665cffd to
4574cb7
Compare
8976066 to
680fbf2
Compare
…ema (#3420) ## Summary: PR series to add logarithm graph support to the Interactive Graph widget: 1.▶️ [Add logarithm graph type definitions and data](#3420) 2. [Add logarithm math utilities to kmath](#3421) 3. [Add logarithm graph state management and reducer](#3422) 4. [Add logarithm graph rendering, SR strings, and equation string](#3423) 5. [Add logarithm graph scoring](#3424) 6. [Add logarithm graph option in the Interactive Graph Editor](#3425) Add logarithm type definitions, this is the initial implementation for supporting Logarithm graph in Interactive Graph widget. - Add `PerseusGraphTypeLogarithm` and `LogarithmGraphCorrect` types to the data schema, following the exponential pattern (coords + asymptote) - Add JSON parser for the new `"logarithm"` graph type - Add `generateIGLogarithmGraph()` test data generator - Add placeholder cases in all exhaustiveness switches so the build stays green ## Details This is the first PR in the logarithm interactive graph series (LEMS-3950). It adds the type definitions with zero runtime behavior change — no graph renders, no scoring, no editor UI. **Data shape:** Logarithm follows the exponential pattern — two curve points plus an asymptote value. For logarithm the asymptote is the x-value of a vertical line (vs. exponential's y-value for a horizontal line). **Placeholder cases** were added to satisfy `UnreachableCaseError` exhaustiveness in: - `interactive-graph-editor.tsx` — graph merging switch - `start-coords/util.ts` — `shouldShowStartCoordsUI` (returns `false`), `getDefaultGraphStartCoords` (returns `undefined`) - `interactive-graph.tsx` — `getEquationString` (returns `""` — safe no-op since this is called unconditionally in the editor render path) - `initialize-graph-state.ts` — `initializeGraphState` (returns `type: "none"`) - `interactive-graph-ai-utils.ts` — `getGraphOptionsForProps` and `getUserInput` These placeholders will be replaced with real implementations in subsequent PRs. **Why `getEquationString` returns `""` instead of throwing:** `getEquationString` is called unconditionally in the editor's `render()` method (`interactive-graph-editor.tsx`). Since this PR adds the parser that accepts `type: "logarithm"`, any content with `correct.type === "logarithm"` (e.g. created via API) would reach the editor and crash the React render if the placeholder threw. All other placeholders in this PR are safe (return no-op values), but `getEquationString` is uniquely on the render path, so it must return a safe value. Future graph types should follow the same pattern: return `""` here, not throw. Issue: LEMS-3953 Co-Authored by Claude Code (Opus) ## Test plan - [ ] `pnpm tsc` passes - [ ] `pnpm knip` passes - [ ] `pnpm lint` passes - [ ] `pnpm prettier . --check` passes - [ ] Generator tests pass (2 new tests: default graph, graph with all props) - [ ] Parser regression tests pass (170 snapshots unchanged) - [ ] Interactive graph tests pass (199 tests) - [ ] Verify no runtime behavior change in Storybook (existing graph types still work) Author: ivyolamit Reviewers: claude[bot], ivyolamit, SonicScrewdriver, handeyeco Required Reviewers: Approved By: SonicScrewdriver Checks: ⏭️ 1 check has been skipped, ✅ 10 checks were successful Pull Request URL: #3420
## Summary: PR series to add logarithm graph support to the Interactive Graph widget: 1. [Add logarithm graph type definitions and data](#3420) 2.▶️ [Add logarithm math utilities to kmath](#3421) 3. [Add logarithm graph state management and reducer](#3422) 4. [Add logarithm graph rendering, SR strings, and equation string](#3423) 5. [Add logarithm graph scoring](#3424) 6. [Add logarithm graph option in the Interactive Graph Editor](#3425) Add the logarithm math utilities to kmath for supporting Logarithm graph in Interactive Graph - Add `LogarithmCoefficient` type and `getLogarithmCoefficients()` to kmath, following the exponential pattern - Compute coefficients `{a, b, c}` for `f(x) = a·ln(b·x + c)` using the inverse exponential approach - No canonical normalization needed (logarithm has no periodic equivalences, same as exponential) ## Details This adds the shared math utility for logarithm coefficient computation to `@khanacademy/kmath`, following the same pattern as `getExponentialCoefficients()`. Both the rendering component (PR 4) and scoring (PR 5) will consume this function. **Mathematical approach (inverse exponential):** 1. Flip each coordinate `(x, y) → (y, x)` — treating the logarithm as the inverse of an exponential 2. Use the asymptote x-value as the flipped exponential's `c` coefficient 4. Compute exponential coefficients `aExp`, `bExp` from the flipped points 5. Invert to get logarithm coefficients: `a = 1/bExp`, `b = 1/aExp`, `c = -cExp/aExp` **Validation guards** (returns `undefined` for invalid inputs): - Same y-coordinate on both points (makes `bExp` undefined) - A point lying on the asymptote - Points on opposite sides of the asymptote - Non-finite or zero intermediate results This matches the reference implementation in `packages/perseus-core/src/utils/grapher-util.ts` (the Grapher widget's `Logarithm` object, lines 449–558). Co-Authored by Claude Code (Opus) Issue: LEMS-3953 ## Test plan - [ ] `pnpm tsc` passes - [ ] `pnpm knip` passes - [ ] `pnpm lint` passes - [ ] `pnpm prettier . --check` passes - [ ] Coefficient tests pass (6 new tests, 17 total in coefficients.test.ts) - Grapher test data: `[-4,-3]`, `[-5,-7]`, asymptote `-6` reproduces correct y-values - Natural log: `[1,0]`, `[e,1]`, asymptote `0` → `a≈1, b≈1, c≈0` - Negative b: points left of asymptote (`y = ln(-x)`) → `b≈-1` - Same-y → `undefined` - Point on asymptote → `undefined` - Opposite sides → `undefined` Author: ivyolamit Reviewers: claude[bot], SonicScrewdriver, handeyeco Required Reviewers: Approved By: SonicScrewdriver Checks: ⏭️ 1 check has been skipped, ✅ 10 checks were successful, ⚪️ 1 check is neutral Pull Request URL: #3421
…3422) ## Summary: PR series to add logarithm graph support to the Interactive Graph widget: 1. [Add logarithm graph type definitions and data](#3420) 2. [Add logarithm math utilities to kmath](#3421) 3.▶️ [Add logarithm graph state management and reducer](#3422) 4. [Add logarithm graph rendering, SR strings, and equation string](#3423) 5. [Add logarithm graph scoring](#3424) 6. [Add logarithm graph option in the Interactive Graph Editor](#3425) Add logarithm graph state management and reducer for supporting Logarithm graph in Interactive Graph - Add `LogarithmGraphState` to the internal state type system - Wire up reducer actions (`movePoint` + `moveCenter`) with logarithm-specific constraints - Add graph state initialization with sensible defaults - Add test data fixtures and question builder support ## Details This PR adds the state management layer for logarithm graphs, following the exponential pattern throughout. Logarithm is the vertical-asymptote mirror of exponential's horizontal-asymptote design. **Action registration:** Reuses existing `movePoint` and `moveCenter` action creators (no new action types). The `actions` export object gets `logarithm: { movePoint, moveCenter }`, identical to exponential. **Reducer — `doMovePoint`:** - Point cannot land on the asymptote's x-coordinate - Both points must have different y-values (prevents degenerate coefficient computation) - Cross-asymptote reflection: when a point is dragged past the asymptote, the other point is reflected (`reflectedX = 2 * asymptoteX - otherX`) so both points end up on the same side — matches Grapher widget behavior **Reducer — `doMoveCenter`:** - Asymptote moves horizontally only (X component extracted, Y ignored) - Snap-through logic: when the new position would land between or on the curve points, snaps past all points using the midpoint heuristic for direction detection (prevents oscillation/flicker) - Final safety check: asymptote cannot land exactly on either point's x-coordinate **Initialization:** `getLogarithmCoords()` follows `getExponentialCoords()` pattern — returns `{coords, asymptote}`. Default coords use normalized fractions `[0.55, 0.55]` and `[0.75, 0.75]` to ensure both points are to the right of the default asymptote at x=0 after normalization (x=0.5 would land exactly on the asymptote). **Placeholders:** `mafs-graph.tsx` returns `{graph: null, interactiveElementsDescription: null}` for logarithm (replaced in PR 4). `mafs-state-to-interactive-graph.ts` has the real serialization (not a placeholder). Co-Authored by Claude Code (Opus) Issue: LEMS-3953 ## Test plan: - [ ] `pnpm tsc` passes - [ ] `pnpm knip` passes - [ ] `pnpm lint` passes - [ ] `pnpm prettier . --check` passes - [ ] Reducer tests pass (147 total, 14 new logarithm tests): - `movePoint`: same-y rejection, bounding-to-same-y rejection, valid move, on-asymptote rejection, cross-asymptote reflection - `moveCenter`: valid move, snap-through between points, Y-component ignored, final safety rejection - Initialization: given coords, startCoords, defaults - Gradable graph: logarithm state conversion - Serialization: logarithm state to interactive graph - [ ] Interactive graph tests pass (206 total, logarithm added to parameterized test maps) - [ ] Serialization tests pass (14 total) Author: ivyolamit Reviewers: claude[bot], ivyolamit, SonicScrewdriver Required Reviewers: Approved By: SonicScrewdriver Checks: ⏭️ 1 check has been skipped, ✅ 10 checks were successful, ⚪️ 1 check is neutral Pull Request URL: #3422
4574cb7 to
dd49d3a
Compare
…uation string (#3423) ## Summary: PR series to add logarithm graph support to the Interactive Graph widget: 1. [Add logarithm graph type definitions and data](#3420) 2. [Add logarithm math utilities to kmath](#3421) 3. [Add logarithm graph state management and reducer](#3422) 4.▶️ [Add logarithm graph rendering, SR strings, and equation string](#3423) 5. [Add logarithm graph scoring](#3424) 6. [Add logarithm graph option in the Interactive Graph Editor](#3425) Create the logarithm graph visual component, add Storybook coverage, SR strings, and equation string for supporting Logarithm graph in Interactive Graph - Create the logarithm graph visual component (`logarithm.tsx`) with curve rendering, draggable asymptote, and movable points - Add 6 screen reader strings for accessibility - Add equation string display for the editor - Add Storybook story ## Details This is the largest PR in the series. It creates the visual rendering of the logarithm graph, following the exponential component pattern with the axis swapped (vertical asymptote instead of horizontal). **Curve rendering:** - Single `<Plot.OfX>` with domain restricted to one side of the asymptote (`[asymptoteX + 0.001, xMax]` or `[xMin, asymptoteX - 0.001]`) - Plot function computes `a * ln(b*x + c)`, returning NaN when outside domain or when y-value exceeds visible range + padding (prevents curve visually touching the asymptote) - `coeffRef` caches last valid coefficients as fallback during transient invalid states **Asymptote rendering:** - Uses existing `MovableAsymptote` component with `orientation="vertical"` - Dispatches `actions.logarithm.moveCenter()` on drag - `constrainAsymptoteKeyboard()` implements snap-through logic for keyboard navigation (mirrors exponential's vertical version but on X-axis) **Keyboard constraints:** - `getLogarithmKeyboardConstraint()` prevents points from landing on the asymptote's x-coordinate or sharing y-value with the other point - Uses bounded retry (max 3 steps) to skip past invalid positions **Equation string:** - `getLogarithmEquationString()` displays `y = a*ln(b*x + c)` with computed coefficient values - `defaultLogarithmCoords()` provides fallback coords (normalized fractions `[0.55, 0.55]`, `[0.75, 0.75]`) **Screen reader strings (6):** - `srLogarithmGraph` — graph container label - `srLogarithmPoint1` / `srLogarithmPoint2` — point position labels - `srLogarithmAsymptote` — asymptote label with keyboard instructions - `srLogarithmDescription` — graph state description - `srLogarithmInteractiveElements` — interactive elements summary Co-Authored by Claude Code (Opus) Issue: LEMS-3953 ## Test plan: - [ ] `pnpm tsc` passes - [ ] `pnpm knip` passes - [ ] `pnpm lint` passes - [ ] `pnpm prettier . --check` passes - [ ] Logarithm component tests pass (15 new tests): - 9 screen reader tests (aria-labels, descriptions, interactive elements, updates on state change) - 3 keyboard constraint tests for points (valid move, skip asymptote, skip same-y) - 3 keyboard constraint tests for asymptote (free move, snap-through, skip point x-value) - [ ] Interactive graph tests pass (178 passed, 28 skipped) - [ ] Verify rendering in Storybook (logarithm story renders correctly, curve matches expected shape) Author: ivyolamit Reviewers: claude[bot], ivyolamit, SonicScrewdriver Required Reviewers: Approved By: SonicScrewdriver Checks: ⏭️ 1 check has been skipped, ✅ 10 checks were successful Pull Request URL: #3423
…h scoring to support the Logarithm graph in Interactive Graph
680fbf2 to
886aa19
Compare
SonicScrewdriver
left a comment
There was a problem hiding this comment.
I ALSO APPROVE AGAIN!
…ph Editor (#3425) ## Summary: PR series to add logarithm graph support to the Interactive Graph widget: 1. [Add logarithm graph type definitions and data](#3420) 2. [Add logarithm math utilities to kmath](#3421) 3. [Add logarithm graph state management and reducer](#3422) 4. [Add logarithm graph rendering, SR strings, and equation string](#3423) 5. [Add logarithm graph scoring](#3424) 6.▶️ [Add logarithm graph option in the Interactive Graph Editor](#3425) Add logarithm graph option in the Interactive Graph Editor - Add `StartCoordsLogarithm` component for configuring logarithm start coordinates in the editor - Gate the "Logarithm function" option behind the `interactive-graph-logarithm` feature flag - Export `getLogarithmCoords` for use by the editor - Add start asymptote validation ## Details This PR enables content creators to configure logarithm exercises in the Interactive Graph editor, following the exponential editor pattern. **StartCoordsLogarithm component:** - Two coordinate pair inputs (Point 1 and Point 2) - A single number input for the asymptote x-position (labeled "Asymptote x =") - Equation display showing `y = a * ln(b*x + c)` with computed coefficient values - CSS module styling (not Aphrodite), following `start-coords-exponential.module.css` **Feature flag:** "Logarithm function" appears in the graph type selector only when `interactive-graph-logarithm` is enabled, preventing content creators from selecting it until it's ready for launch. **Editor validation:** The start asymptote x-value is validated — it cannot fall between or on the x-coordinates of the curve's start points (mirrors exponential's y-axis validation but for x-axis). **Exports:** `getLogarithmCoords()` is now exported from `initialize-graph-state.ts` and re-exported from `@khanacademy/perseus` for use by the editor's start-coords UI. Co-Authored by Claude Code (Opus) Issue: LEMS-3969 ## Test plan: - [ ] `pnpm tsc` passes - [ ] `pnpm knip` passes - [ ] `pnpm lint` passes - [ ] `pnpm prettier . --check` passes - [ ] Editor tests pass (422 total across 15 suites) - [ ] Verify in Storybook: "Logarithm function" appears in graph type selector when feature flag is on - [ ] Verify start coords UI renders correctly with asymptote input and equation display Author: ivyolamit Reviewers: claude[bot], SonicScrewdriver, ivyolamit Required Reviewers: Approved By: SonicScrewdriver Checks: ⏭️ 1 check has been skipped, ✅ 10 checks were successful Pull Request URL: #3425
This PR was opened by the [Changesets release](https://github.com/changesets/action) GitHub action. When you're ready to do a release, you can merge this and the packages will be published to npm automatically. If you're not ready to do a release yet, that's fine, whenever you add more changesets to main, this PR will be updated. # Releases ## @khanacademy/kmath@2.4.0 ### Minor Changes - [#3421](#3421) [`9099a40a4e`](9099a40) Thanks [@ivyolamit](https://github.com/ivyolamit)! - Add the logarithm math utilities to kmath for supporting Logarithm graph in Interactive Graph ### Patch Changes - Updated dependencies \[[`a72f7db4a3`](a72f7db)]: - @khanacademy/perseus-core@24.1.0 ## @khanacademy/perseus@77.2.0 ### Minor Changes - [#3422](#3422) [`a4eaa5a1f8`](a4eaa5a) Thanks [@ivyolamit](https://github.com/ivyolamit)! - Add logarithm graph state management and reducer for supporting Logarithm graph in Interactive Graph - [#3425](#3425) [`2c36b8a4c3`](2c36b8a) Thanks [@ivyolamit](https://github.com/ivyolamit)! - Add logarithm graph option in the Interactive Graph Editor - [#3423](#3423) [`3cc56f60dd`](3cc56f6) Thanks [@ivyolamit](https://github.com/ivyolamit)! - Create the logarithm graph visual component, add Storybook coverage, SR strings, and equation string for supporting Logarithm graph in Interactive Graph - [#3420](#3420) [`a72f7db4a3`](a72f7db) Thanks [@ivyolamit](https://github.com/ivyolamit)! - Add logarithm type definitions, this is the initial implementation for supporting Logarithm graph in Interactive Graph widget. ### Patch Changes - [#3461](#3461) [`0481b4238d`](0481b42) Thanks [@nishasy](https://github.com/nishasy)! - [Image] | (DX) | Update CSS modules to kabab case - [#3455](#3455) [`0d6cf3c3ce`](0d6cf3c) Thanks [@nishasy](https://github.com/nishasy)! - [Image] | (UX) | Fix messed up Graphie labels behind scale feature flag - Updated dependencies \[[`08b1218b62`](08b1218), [`a72f7db4a3`](a72f7db), [`9099a40a4e`](9099a40)]: - @khanacademy/perseus-score@8.6.0 - @khanacademy/perseus-core@24.1.0 - @khanacademy/kmath@2.4.0 - @khanacademy/keypad-context@3.2.42 - @khanacademy/math-input@26.4.13 - @khanacademy/perseus-linter@4.9.2 ## @khanacademy/perseus-core@24.1.0 ### Minor Changes - [#3420](#3420) [`a72f7db4a3`](a72f7db) Thanks [@ivyolamit](https://github.com/ivyolamit)! - Add logarithm type definitions, this is the initial implementation for supporting Logarithm graph in Interactive Graph widget. ## @khanacademy/perseus-editor@30.2.0 ### Minor Changes - [#3425](#3425) [`2c36b8a4c3`](2c36b8a) Thanks [@ivyolamit](https://github.com/ivyolamit)! - Add logarithm graph option in the Interactive Graph Editor - [#3420](#3420) [`a72f7db4a3`](a72f7db) Thanks [@ivyolamit](https://github.com/ivyolamit)! - Add logarithm type definitions, this is the initial implementation for supporting Logarithm graph in Interactive Graph widget. ### Patch Changes - [#3461](#3461) [`0481b4238d`](0481b42) Thanks [@nishasy](https://github.com/nishasy)! - [Image] | (DX) | Update CSS modules to kabab case - Updated dependencies \[[`a4eaa5a1f8`](a4eaa5a), [`2c36b8a4c3`](2c36b8a), [`3cc56f60dd`](3cc56f6), [`08b1218b62`](08b1218), [`a72f7db4a3`](a72f7db), [`9099a40a4e`](9099a40), [`0481b4238d`](0481b42), [`0d6cf3c3ce`](0d6cf3c)]: - @khanacademy/perseus@77.2.0 - @khanacademy/perseus-score@8.6.0 - @khanacademy/perseus-core@24.1.0 - @khanacademy/kmath@2.4.0 - @khanacademy/keypad-context@3.2.42 - @khanacademy/math-input@26.4.13 - @khanacademy/perseus-linter@4.9.2 ## @khanacademy/perseus-score@8.6.0 ### Minor Changes - [#3424](#3424) [`08b1218b62`](08b1218) Thanks [@ivyolamit](https://github.com/ivyolamit)! - Add logarithm graph scoring to support the Logarithm graph in Interactive Graph ### Patch Changes - Updated dependencies \[[`a72f7db4a3`](a72f7db), [`9099a40a4e`](9099a40)]: - @khanacademy/perseus-core@24.1.0 - @khanacademy/kmath@2.4.0 ## @khanacademy/keypad-context@3.2.42 ### Patch Changes - Updated dependencies \[[`a72f7db4a3`](a72f7db)]: - @khanacademy/perseus-core@24.1.0 ## @khanacademy/math-input@26.4.13 ### Patch Changes - Updated dependencies \[[`a72f7db4a3`](a72f7db)]: - @khanacademy/perseus-core@24.1.0 - @khanacademy/keypad-context@3.2.42 ## @khanacademy/perseus-linter@4.9.2 ### Patch Changes - Updated dependencies \[[`a72f7db4a3`](a72f7db), [`9099a40a4e`](9099a40)]: - @khanacademy/perseus-core@24.1.0 - @khanacademy/kmath@2.4.0
Summary:
PR series to add logarithm graph support to the Interactive Graph widget:
Add logarithm graph scoring to support the Logarithm graph in Interactive Graph
score-interactive-graph.tsusing direct coefficient comparisonDetails
Follows the exponential scoring pattern exactly. Computes
{a, b, c}coefficients for both the user's answer and the rubric usinggetLogarithmCoefficientsfrom kmath, then compares withapproximateDeepEqual.Scoring logic:
invalidif guess coords or asymptote are missing, or if coefficient computation fails for either sideearned: 1if[a, b, c]coefficients approximately matchearned: 0otherwiseThis means two different sets of control points that define the same logarithmic curve will score as correct — the comparison is on the mathematical function, not the specific points chosen.
Co-Authored by Claude Code (Opus)
Issue: LEMS-3953
Test plan:
pnpm tscpassespnpm knippassespnpm lintpassespnpm prettier . --checkpassesy = ln(x)coefficients → correct